Skip to content

[codex] Add release sanity checks#491

Merged
k82cn merged 2 commits into
xflops:mainfrom
k82cn:codex/release-sanity-check
Jun 4, 2026
Merged

[codex] Add release sanity checks#491
k82cn merged 2 commits into
xflops:mainfrom
k82cn:codex/release-sanity-check

Conversation

@k82cn

@k82cn k82cn commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add ci/release/sanity.sh for non-publishing release validation.
  • Add a make release-sanity target.
  • Document the Docker Compose release smoke check that pulls the target image tag and verifies flamepy from PyPI in a clean Python image.

Validation

  • bash -n ci/release/sanity.sh
  • RELEASE_SANITY_LOCAL_CHECKS=0 RELEASE_SANITY_PACKAGE_CHECKS=0 RELEASE_SANITY_REMOTE_CHECKS=0 RELEASE_SANITY_K8S_E2E=0 RELEASE_SANITY_COMPOSE_E2E=0 make release-sanity
  • CONTAINER_RUNTIME=podman RELEASE_TAG=v0.6.0-rc2 CARGO_VERSION=0.6.0-rc2 PYTHON_VERSION=0.6.0rc2 DOCKER_TAG=v0.6.0-rc2 IMAGE_REGISTRY=docker.io/xflops RELEASE_SANITY_METADATA_CHECKS=0 RELEASE_SANITY_LOCAL_CHECKS=0 RELEASE_SANITY_PACKAGE_CHECKS=0 RELEASE_SANITY_REMOTE_CHECKS=0 RELEASE_SANITY_K8S_E2E=0 RELEASE_SANITY_COMPOSE_E2E=1 RELEASE_SANITY_COMPOSE_E2E_TASKS=1 RELEASE_SANITY_COMPOSE_TIMEOUT_SECONDS=300 make release-sanity

The compose smoke pulled docker.io/xflops/flame-*:{v0.6.0-rc2}, found the flmrun template, installed flamepy==0.6.0rc2 from PyPI in python:3.12-slim, imported it from /usr/local/lib/python3.12/site-packages, and completed python -m flamepy.runner.e2e --tasks 1 --json.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new release sanity check script (ci/release/sanity.sh), integrates it into the Makefile via the release-sanity target, and updates the release process documentation. The script automates local checks, package validation, remote artifact verification, and Docker Compose end-to-end smoke tests. Feedback on the changes focuses on security and reliability improvements in the shell script. Specifically, the reviewer recommends replacing insecure temporary file creation (using $$ in /tmp) with mktemp or direct evaluation, fixing a logic bug where platform validation failures are silently ignored due to && chaining, redirecting Helm template output to /dev/null to avoid shared environment conflicts, and ensuring all temporary files are properly cleaned up in the exit trap handler.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ci/release/sanity.sh
Comment on lines +85 to +166
load_detected_versions() {
need python3

local versions_file
versions_file="${TMPDIR:-/tmp}/flame-release-sanity-versions.$$"

python3 - "$ROOT_DIR" >"$versions_file" <<'PY'
import ast
import pathlib
import shlex
import sys

try:
import tomllib
except ModuleNotFoundError as exc:
raise SystemExit("python3 with tomllib is required") from exc

root = pathlib.Path(sys.argv[1])


def load_toml(path):
with path.open("rb") as f:
return tomllib.load(f)


def shell_var(name, value):
if not value:
raise SystemExit(f"missing value for {name}")
print(f"{name}={shlex.quote(str(value))}")


def package_version(path):
data = load_toml(path)
return data["package"]["version"]


def pyproject_version(path):
data = load_toml(path)
return data["project"]["version"]


def python_init_version(path):
tree = ast.parse(path.read_text())
for node in tree.body:
if not isinstance(node, ast.Assign):
continue
for target in node.targets:
if isinstance(target, ast.Name) and target.id == "__version__":
if isinstance(node.value, ast.Constant) and isinstance(node.value.value, str):
return node.value.value
raise SystemExit(f"missing __version__ assignment in {path}")


def chart_top_level(path, key):
prefix = f"{key}:"
for line in path.read_text().splitlines():
if line.startswith((" ", "\t", "#")):
continue
stripped = line.strip()
if stripped.startswith(prefix):
return stripped.split(":", 1)[1].strip().strip("\"'")
raise SystemExit(f"missing {key} in {path}")


sdk_rust = load_toml(root / "sdk/rust/Cargo.toml")
stdng_dep = sdk_rust["dependencies"]["stdng"]
macros_dep = sdk_rust["dependencies"]["flame-rs-macros"]

shell_var("DETECTED_PYPROJECT_VERSION", pyproject_version(root / "sdk/python/pyproject.toml"))
shell_var("DETECTED_PYTHON_INIT_VERSION", python_init_version(root / "sdk/python/src/flamepy/__init__.py"))
shell_var("DETECTED_FLAME_RS_VERSION", package_version(root / "sdk/rust/Cargo.toml"))
shell_var("DETECTED_FLAME_RS_MACROS_VERSION", package_version(root / "sdk/rust/macros/Cargo.toml"))
shell_var("DETECTED_STDNG_VERSION", package_version(root / "stdng/Cargo.toml"))
shell_var("DETECTED_CHART_APP_VERSION", chart_top_level(root / "charts/flame/Chart.yaml", "appVersion"))
shell_var("DETECTED_FLAME_RS_STDNG_DEP_VERSION", stdng_dep.get("version"))
shell_var("DETECTED_FLAME_RS_MACROS_DEP_VERSION", macros_dep.get("version"))
PY

# shellcheck disable=SC1090
. "$versions_file"
rm -f "$versions_file"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of load_detected_versions creates a temporary file in /tmp using the process ID ($$), which is insecure and can lead to symlink attacks or conflicts in shared environments. Additionally, if the script fails before the file is deleted, it leaves orphaned files behind.

We can completely avoid creating any temporary files by directly evaluating the stdout of the Python script using eval. Since the Python script is a hardcoded heredoc and safely quotes its output using shlex.quote, this is both secure and highly efficient.

load_detected_versions() {
    need python3

    eval "$(python3 - "$ROOT_DIR" <<'PY'
import ast
import pathlib
import shlex
import sys

try:
    import tomllib
except ModuleNotFoundError as exc:
    raise SystemExit("python3 with tomllib is required") from exc

root = pathlib.Path(sys.argv[1])


def load_toml(path):
    with path.open("rb") as f:
        return tomllib.load(f)


def shell_var(name, value):
    if not value:
        raise SystemExit(f"missing value for {name}")
    print(f"{name}={shlex.quote(str(value))}")


def package_version(path):
    data = load_toml(path)
    return data["package"]["version"]


def pyproject_version(path):
    data = load_toml(path)
    return data["project"]["version"]


def python_init_version(path):
    tree = ast.parse(path.read_text())
    for node in tree.body:
        if not isinstance(node, ast.Assign):
            continue
        for target in node.targets:
            if isinstance(target, ast.Name) and target.id == "__version__":
                if isinstance(node.value, ast.Constant) and isinstance(node.value.value, str):
                    return node.value.value
    raise SystemExit(f"missing __version__ assignment in {path}")


def chart_top_level(path, key):
    prefix = f"{key}:"
    for line in path.read_text().splitlines():
        if line.startswith((" ", "\t", "#")):
            continue
        stripped = line.strip()
        if stripped.startswith(prefix):
            return stripped.split(":", 1)[1].strip().strip("\"'")
    raise SystemExit(f"missing {key} in {path}")


sdk_rust = load_toml(root / "sdk/rust/Cargo.toml")
stdng_dep = sdk_rust["dependencies"]["stdng"]
macros_dep = sdk_rust["dependencies"]["flame-rs-macros"]

shell_var("DETECTED_PYPROJECT_VERSION", pyproject_version(root / "sdk/python/pyproject.toml"))
shell_var("DETECTED_PYTHON_INIT_VERSION", python_init_version(root / "sdk/python/src/flamepy/__init__.py"))
shell_var("DETECTED_FLAME_RS_VERSION", package_version(root / "sdk/rust/Cargo.toml"))
shell_var("DETECTED_FLAME_RS_MACROS_VERSION", package_version(root / "sdk/rust/macros/Cargo.toml"))
shell_var("DETECTED_STDNG_VERSION", package_version(root / "stdng/Cargo.toml"))
shell_var("DETECTED_CHART_APP_VERSION", chart_top_level(root / "charts/flame/Chart.yaml", "appVersion"))
shell_var("DETECTED_FLAME_RS_STDNG_DEP_VERSION", stdng_dep.get("version"))
shell_var("DETECTED_FLAME_RS_MACROS_DEP_VERSION", macros_dep.get("version"))
PY
)"
}

Comment thread ci/release/sanity.sh
Comment on lines +336 to +373
inspect_image_manifest() {
local image="$1"
local manifest_file

manifest_file="${TMPDIR:-/tmp}/flame-release-sanity-manifest.$$.$(basename "$image").json"

if command -v docker >/dev/null 2>&1; then
log "docker buildx imagetools inspect --raw ${image}"
if docker buildx imagetools inspect --raw "$image" >"$manifest_file" 2>/dev/null && check_manifest_platforms "$image" "$manifest_file"; then
rm -f "$manifest_file"
return
fi
fi

if command -v podman >/dev/null 2>&1; then
log "podman manifest inspect ${image}"
if podman manifest inspect "$image" >"$manifest_file" 2>/dev/null && check_manifest_platforms "$image" "$manifest_file"; then
rm -f "$manifest_file"
return
fi

log "podman pull ${image}"
podman pull "$image" >/dev/null
log "podman image inspect ${image}"
podman image inspect "$image" >"$manifest_file"
check_manifest_platforms "$image" "$manifest_file"
elif command -v docker >/dev/null 2>&1; then
log "docker pull ${image}"
docker pull "$image" >/dev/null
log "docker image inspect ${image}"
docker image inspect "$image" >"$manifest_file"
check_manifest_platforms "$image" "$manifest_file"
else
fail "podman or docker is required for image manifest checks"
fi

rm -f "$manifest_file"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two issues here:

  1. Creating a temporary file using $$ in /tmp is insecure and prone to symlink attacks or race conditions. We should use mktemp instead.
  2. Because check_manifest_platforms is chained with && inside the if condition, any validation failure (e.g., missing expected platforms) will be silently ignored by the if statement, causing the script to fall back to other tools (like pulling the image locally) instead of failing immediately. Since a successful manifest inspect is definitive, any platform mismatch should fail the script immediately.

We can resolve both issues by using mktemp and separating the inspection command from the validation check.

inspect_image_manifest() {
    local image="$1"

    MANIFEST_FILE="$(mktemp "${TMPDIR:-/tmp}/flame-release-sanity-manifest.XXXXXX")"

    if command -v docker >/dev/null 2>&1; then
        log "docker buildx imagetools inspect --raw ${image}"
        if docker buildx imagetools inspect --raw "$image" >"$MANIFEST_FILE" 2>/dev/null; then
            check_manifest_platforms "$image" "$MANIFEST_FILE"
            rm -f "$MANIFEST_FILE"
            MANIFEST_FILE=""
            return
        fi
    fi

    if command -v podman >/dev/null 2>&1; then
        log "podman manifest inspect ${image}"
        if podman manifest inspect "$image" >"$MANIFEST_FILE" 2>/dev/null; then
            check_manifest_platforms "$image" "$MANIFEST_FILE"
            rm -f "$MANIFEST_FILE"
            MANIFEST_FILE=""
            return
        fi

        log "podman pull ${image}"
        podman pull "$image" >/dev/null
        log "podman image inspect ${image}"
        podman image inspect "$image" >"$MANIFEST_FILE"
        check_manifest_platforms "$image" "$MANIFEST_FILE"
    elif command -v docker >/dev/null 2>&1; then
        log "docker pull ${image}"
        docker pull "$image" >/dev/null
        log "docker image inspect ${image}"
        docker image inspect "$image" >"$MANIFEST_FILE"
        check_manifest_platforms "$image" "$MANIFEST_FILE"
    else
        fail "podman or docker is required for image manifest checks"
    fi

    rm -f "$MANIFEST_FILE"
    MANIFEST_FILE=""
}

Comment thread ci/release/sanity.sh
Comment on lines +60 to +73
cleanup() {
if [ "$COMPOSE_STARTED" = "1" ] && enabled "$COMPOSE_DOWN"; then
log "Stopping Docker Compose project ${COMPOSE_PROJECT_NAME}"
compose_command down -v || true
fi

if [ -n "$COMPOSE_OVERRIDE_FILE" ]; then
rm -f "$COMPOSE_OVERRIDE_FILE"
fi

if [ -n "$PYPI_CHECK_SCRIPT" ]; then
rm -f "$PYPI_CHECK_SCRIPT"
fi
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure that the temporary manifest file created during remote checks is always cleaned up (even if the script exits prematurely due to a validation failure), we should add it to the cleanup function.

Suggested change
cleanup() {
if [ "$COMPOSE_STARTED" = "1" ] && enabled "$COMPOSE_DOWN"; then
log "Stopping Docker Compose project ${COMPOSE_PROJECT_NAME}"
compose_command down -v || true
fi
if [ -n "$COMPOSE_OVERRIDE_FILE" ]; then
rm -f "$COMPOSE_OVERRIDE_FILE"
fi
if [ -n "$PYPI_CHECK_SCRIPT" ]; then
rm -f "$PYPI_CHECK_SCRIPT"
fi
}
cleanup() {
if [ "$COMPOSE_STARTED" = "1" ] && enabled "$COMPOSE_DOWN"; then
log "Stopping Docker Compose project ${COMPOSE_PROJECT_NAME}"
compose_command down -v || true
fi
if [ -n "$COMPOSE_OVERRIDE_FILE" ]; then
rm -f "$COMPOSE_OVERRIDE_FILE"
fi
if [ -n "$PYPI_CHECK_SCRIPT" ]; then
rm -f "$PYPI_CHECK_SCRIPT"
fi
if [ -n "${MANIFEST_FILE:-}" ]; then
rm -f "$MANIFEST_FILE"
fi
}

Comment thread ci/release/sanity.sh Outdated
Comment on lines +240 to +243
log "helm template flame charts/flame"
helm template flame "$ROOT_DIR/charts/flame" \
--set "global.imageRegistry=${IMAGE_REGISTRY}" \
--set "global.imageTag=${DOCKER_TAG}" >/tmp/flame-release-sanity-rendered.yaml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Writing the rendered Helm template to a hardcoded path /tmp/flame-release-sanity-rendered.yaml can cause permission conflicts if multiple users run the script on the same machine. Since the goal is only to verify that helm template succeeds without errors, we can safely redirect the output to /dev/null.

Suggested change
log "helm template flame charts/flame"
helm template flame "$ROOT_DIR/charts/flame" \
--set "global.imageRegistry=${IMAGE_REGISTRY}" \
--set "global.imageTag=${DOCKER_TAG}" >/tmp/flame-release-sanity-rendered.yaml
log "helm template flame charts/flame"
helm template flame "$ROOT_DIR/charts/flame" \
--set "global.imageRegistry=${IMAGE_REGISTRY}" \
--set "global.imageTag=${DOCKER_TAG}" >/dev/null

Comment thread ci/release/sanity.sh
Comment on lines +453 to +455
write_compose_override() {
COMPOSE_OVERRIDE_FILE="${TMPDIR:-/tmp}/flame-release-sanity-compose.$$.yaml"
cat >"$COMPOSE_OVERRIDE_FILE" <<EOF

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use mktemp instead of process ID ($$) to securely create the temporary compose override file.

Suggested change
write_compose_override() {
COMPOSE_OVERRIDE_FILE="${TMPDIR:-/tmp}/flame-release-sanity-compose.$$.yaml"
cat >"$COMPOSE_OVERRIDE_FILE" <<EOF
write_compose_override() {
COMPOSE_OVERRIDE_FILE="$(mktemp "${TMPDIR:-/tmp}/flame-release-sanity-compose.XXXXXX")"
cat >"$COMPOSE_OVERRIDE_FILE" <<EOF

Comment thread ci/release/sanity.sh
Comment on lines +471 to +473
write_pypi_check_script() {
PYPI_CHECK_SCRIPT="${TMPDIR:-/tmp}/flame-release-sanity-pypi-check.$$.sh"
cat >"$PYPI_CHECK_SCRIPT" <<'EOF'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use mktemp instead of process ID ($$) to securely create the temporary PyPI check script.

Suggested change
write_pypi_check_script() {
PYPI_CHECK_SCRIPT="${TMPDIR:-/tmp}/flame-release-sanity-pypi-check.$$.sh"
cat >"$PYPI_CHECK_SCRIPT" <<'EOF'
write_pypi_check_script() {
PYPI_CHECK_SCRIPT="$(mktemp "${TMPDIR:-/tmp}/flame-release-sanity-pypi-check.XXXXXX")"
cat >"$PYPI_CHECK_SCRIPT" <<'EOF'

@k82cn k82cn marked this pull request as ready for review June 4, 2026 01:51
@k82cn k82cn merged commit 35808b0 into xflops:main Jun 4, 2026
8 checks passed
@k82cn k82cn deleted the codex/release-sanity-check branch June 4, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant